Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use brewkit-v1 #4314

Merged
merged 7 commits into from
Dec 13, 2023
Merged

use brewkit-v1 #4314

merged 7 commits into from
Dec 13, 2023

Conversation

mxcl
Copy link
Member

@mxcl mxcl commented Dec 5, 2023 •
edited
Loading

  • ci.yml is pull requests
  • ci-squared runs to verify changes to the deploy yamls can be tested without uploading anything to bottle storage
  • pkger.yml is the workhorse
  • built/test/bottle per platform/arch combo are in same workflow
    • this means that bottles will upload if a platform succeeds rather than all bottles failing if one platform fails

@mxcl mxcl force-pushed the brewkit-v1 branch 6 times, most recently from 9fc4509 to 02977ad Compare December 12, 2023 16:40
@mxcl
Copy link
Member Author

mxcl commented Dec 12, 2023

Not passing because brewkit v1 needs a formal release before this will be green.

jhheider
jhheider reviewed Dec 12, 2023
View reviewed changes
.github/workflows/pkg-platform.yml
Comment on lines +31 to +37
test-container:
description: >
A JSON array of docker image names or `[null]`.
Indeed! You cannot leave this as `null` or undefined.
Sorry, GHA is not flexible enough to efficiently work around this.
required: true
type: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simpler to make it default: '[null]'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I couldn't easily get this to work because the github actions expressions capabilities are too inadequate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ie. you cannot || off an array and you cannot check if the content of a variable is an array either.

jhheider
jhheider reviewed Dec 12, 2023
View reviewed changes
pkgx.yaml
@@ -1,5 +1,4 @@
dependencies:
pkgx.sh/brewkit: ^0
pkgx.sh/brewkit: ^0 || ^1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you like this better than <2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What and allow v−1 😂. I prefer this, but yeah either would be fine.

jhheider
jhheider approved these changes Dec 12, 2023
View reviewed changes
Copy link
Contributor

@jhheider jhheider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this, too, looks good. i am certain there will be some breakage we cannot find until it is live.

@mxcl
Copy link
Member Author

mxcl commented Dec 12, 2023

failure because we have exotic unicode characters in our paths and Ruby 2 doesn’t support unicode paths and this failure is during the brewkit package.yml test that runs bk build lol.

Got a fix in mind.

@jhheider
Copy link
Contributor

Got a fix in mind.

is it not using exotic unicode characters? or am i taking crazy pills? :)

@mxcl
Copy link
Member Author

mxcl commented Dec 12, 2023

Wasn't sure where I should use {group: 'linux-x86-64'} vs [self-hosted, linux, X64]

@mxcl
Copy link
Member Author

mxcl commented Dec 12, 2023 •
edited
Loading

Got a fix in mind.

is it not using exotic unicode characters? or am i taking crazy pills? :)

we swap / for a unicode slash for the user’s readability. We could not do this for the “not building in a pantry checkout case” (which builds to XDG_DATA_HOME) but coincidentally our CI runs in a pantry checkout so it would still happen.

I could make the actions/checkout step always go to a subdir to workaround that and probably should.

Or abandon the slash replacement altogether. It's nice to be able to easily browse the various folders in a pantry checkout, but in general I haven't felt it required for usage.

@jhheider
Copy link
Contributor

Wasn't sure where I should use {group: 'linux-x86-64'} vs [self-hosted, linux, X64]

we moved off of [self-hosted, linux, X64] due to blocking. it was the slowest, by a wide margin, and singular. by using 4GB GHA instances, its still kind of slow, but it doesn't block.

makes [self-hosted, macOS, X64] our bottleneck, but it's much less of a problem, since it's only for build steps.

@jhheider jhheider linked an issue Dec 12, 2023 that may be closed by this pull request
❌ build issues: pkgx.sh/brewkit=1.0.0 #4375
Closed
@mxcl
Copy link
Member Author

mxcl commented Dec 12, 2023

All green. Will merge tomorrow.

@mxcl mxcl changed the title wip use brewkit-v1 ~wip~ use brewkit-v1 Dec 12, 2023
@mxcl mxcl changed the title ~wip~ use brewkit-v1 use brewkit-v1 Dec 12, 2023
@mxcl mxcl merged commit 7193b3c into main Dec 13, 2023
47 checks passed
@mxcl mxcl deleted the brewkit-v1 branch December 13, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhheider jhheider jhheider approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

❌ build issues: pkgx.sh/brewkit=1.0.0
2 participants
@mxcl @jhheider